sql: render PostgreSQL array literals as ARRAY[...] in unparser#21513
sql: render PostgreSQL array literals as ARRAY[...] in unparser#21513xiedeyantu wants to merge 1 commit intoapache:mainfrom
Conversation
|
@metegenez Could you please help me review this PR? |
|
Sure, tomorrow I'll look at it |
metegenez
left a comment
metegenez
left a comment
There was a problem hiding this comment.
Nice change, this looks correct to me. One request before approving: could you add a test that exercises the second code path you modified?
The current test SELECT [1, 2, 3, 4, 5] only hits the make_array scalar function branch (around line 604). The other hunk you changed (around line 615, the ScalarValue::List branch) isn't covered, since the literal stays as a make_array call through the roundtrip and never reaches that code.
A small unit test that builds an Expr::Literal(ScalarValue::List(...)) directly and calls expr_to_sql under PostgreSqlDialect, asserting the output is ARRAY[1, 2, 3], would pin that line down. A nested case like [[1, 2], [3, 4]] → ARRAY[ARRAY[1, 2], ARRAY[3, 4]] would also be a nice sanity check that recursion through the hook works as expected.
Otherwise LGTM.
| fn identifier_quote_style(&self, _identifier: &str) -> Option<char>; | ||
|
|
||
| /// Whether array literals should be rendered with the `ARRAY[...]` keyword. | ||
| fn array_keyword(&self) -> bool { |
There was a problem hiding this comment.
maybe use_array_keyword_for_array_literals type of name would be more suitable here.
2 participants
Which issue does this PR close?
Rationale for this change
PostgreSQL array literals should be rendered using
ARRAY[...]syntax when unparsing SQL. Without this, roundtripped PostgreSQL SQL can lose the dialect-specific array form and emit a plain bracketed array literal instead.What changes are included in this PR?
ARRAY[...].PostgreSqlDialect.ARRAY[1, 2, 3, 4, 5].Are these changes tested?
Yes. The PostgreSQL roundtrip test in plan_to_sql.rs covers the array literal case and verifies the unparsed SQL now uses
ARRAY[...].Are there any user-facing changes?
Yes. PostgreSQL SQL generated by DataFusion will now emit array literals in
ARRAY[...]form instead of plain bracketed form.